Internal: Remove _ for array lengths#467
Conversation
chelsea0x3b
changed the title
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request correctly fixes a build issue with cargo publish by replacing inferred-length array initializers ([0; _]) with an explicit length ([0; 16]). This is a good fix. However, to further improve the code's robustness and make it more idiomatic, I've suggested using ..Default::default() for struct initialization. This approach is more resilient to future changes in the underlying C API struct definitions, as it automatically handles the initialization of remaining fields, including conditionally compiled ones. This avoids hardcoding the array length and makes the code cleaner and safer.
| flags: 0, | ||
| reserved: [0; _], | ||
| reserved: [0; 16], |
There was a problem hiding this comment.
While hardcoding 16 fixes the build issue, it makes the code brittle. If the reserved field in CUDA_EXTERNAL_MEMORY_HANDLE_DESC ever changes size in a future CUDA version, this code will be incorrect. A more robust and idiomatic way to initialize the remaining fields is to use ..Default::default(). This will correctly zero-initialize flags and reserved and will adapt to future changes in the struct definition.
| flags: 0, | |
| reserved: [0; _], | |
| reserved: [0; 16], | |
| ..Default::default() |
| flags: 0, | ||
| reserved: [0; _], | ||
| reserved: [0; 16], |
There was a problem hiding this comment.
Similar to the previous change, hardcoding 16 here is brittle. Using ..Default::default() is a more robust and idiomatic way to initialize the remaining fields of the struct to their default (zero) values.
| flags: 0, | |
| reserved: [0; _], | |
| reserved: [0; 16], | |
| ..Default::default() |
| flags: 0, | ||
| #[cfg(feature = "cuda-13000")] | ||
| reserved: [0; _], | ||
| reserved: [0; 16], |
There was a problem hiding this comment.
Using ..Default::default() is more robust and idiomatic than manually zeroing fields. It will correctly initialize flags and, when the cuda-13000 feature is enabled, the reserved field. This also makes the code consistent with the import_external_memory_opaque_win32 function in this file.
| flags: 0, | |
| #[cfg(feature = "cuda-13000")] | |
| reserved: [0; _], | |
| reserved: [0; 16], | |
| ..Default::default() |
| flags: 0, | ||
| #[cfg(feature = "cuda-13000")] | ||
| reserved: [0; _], | ||
| reserved: [0; 16], |
There was a problem hiding this comment.
Similar to the other changes, using ..Default::default() here is more robust. It will correctly zero-initialize flags and, when the cuda-13000 feature is enabled, the reserved field, adapting to any future changes in the struct definition.
| flags: 0, | |
| #[cfg(feature = "cuda-13000")] | |
| reserved: [0; _], | |
| reserved: [0; 16], | |
| ..Default::default() |
1 participant
This is for some reason an error when running cargo publish but not cargo clippy? 🤔